Skip to content

my first agent#2

Merged
rockwotj merged 1 commit intomainfrom
agent-one
Apr 2, 2025
Merged

my first agent#2
rockwotj merged 1 commit intomainfrom
agent-one

Conversation

@rockwotj
Copy link
Copy Markdown
Contributor

@rockwotj rockwotj commented Mar 27, 2025

@rockwotj rockwotj marked this pull request as draft March 27, 2025 03:40
@rockwotj rockwotj force-pushed the agent-one branch 4 times, most recently from e3c543a to d170790 Compare March 27, 2025 20:28
@rockwotj rockwotj requested a review from a team March 27, 2025 20:30
@rockwotj rockwotj marked this pull request as ready for review March 27, 2025 20:30
@rockwotj rockwotj force-pushed the agent-one branch 2 times, most recently from f41be0c to d9df5a6 Compare March 28, 2025 14:58
Comment thread src/agents/mcp.py
def __init__(self, directory: Path | str | None, cache_tools: bool = True):
super().__init__(
params=StdioServerParameters(
command="redpanda-connect",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to be redpanda-connect or rather something like rpk connect? If not, what is the redpanda-connect command?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, redpanda-connect is the raw binary from the connect repo that rpk connect invokes as a plugin. This code is certainly going to change as this all evolves

Comment thread src/agents/mcp.py Outdated
from .tools import Tool, ToolResponse, ToolResponseImageContent, ToolResponseTextContent


class MCPServer:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming is a bit confusing. It seems these are all MCP clients and not mcp servers. I assume done for users of this sdk because they think of defining MCP servers as their targets and not necessarily about configuring clients?

How abbout Connection instead of Server?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points. I strugged with the naming here. I used Endpoint because this isn't a live connection just how to connect to a server.

Comment thread src/agents/mcp.py Outdated
"""

# TODO(rockwood): support list change notifications
_cache_tools: bool
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that MCP differentiates between tools, resources etc I wonder whether we should rename to _cache_initialization to make it more generic and cache the entire init response.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just changed to cache_enabled

Comment thread src/agents/mcp.py Outdated

class SSEMCPServer(MCPServer):
"""
A MCP server that communicates with an MCP server over Server-Sent Events.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That description seems weird. Maybe something like: A MCP server that communicates with clients over SSE?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is clearer now using SSEMCPEndpoint

Comment thread src/agents/mcp.py Outdated

class WebsocketMCPServer(MCPServer):
"""
A MCP server that communicates with an MCP server over a WebSocket.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double mention of mcp server

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with Endpoint rename

Comment thread src/agents/agent.py Outdated
@rockwotj rockwotj force-pushed the agent-one branch 2 times, most recently from 38fe3f1 to 36b84de Compare April 2, 2025 14:07
@rockwotj rockwotj merged commit 4fcb04a into main Apr 2, 2025
3 checks passed
@rockwotj rockwotj deleted the agent-one branch April 2, 2025 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants